Repository navigation
Conversation
The msgspec attrs hook factory generated unstructuring hooks while its recursion probe was still active, so handlers for fields leading back to a class being probed were baked in as msgspec.to_builtins. Hook generation is now deferred to the first call whenever a probe is active, so every level of a recursive class goes through the same cattrs hook.
Merging this PR will degrade performance by 11.35%ℹ️ 64 benchmarks have no flame graph
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ❌ ℹ️ | test_unstructure_simple_int_enum[OrjsonConverter] |
16 µs | 18.2 µs | -12.39% |
| ❌ ℹ️ | test_unstructure_simple_int_enum[MsgspecJsonConverter] |
16.1 µs | 18.2 µs | -11.76% |
| ❌ ℹ️ | test_unstructure_simple_int_enum[Converter] |
16.3 µs | 18.4 µs | -11.18% |
| ❌ ℹ️ | test_unstructure_simple_int_enum[BaseConverter] |
17.2 µs | 19.1 µs | -10.05% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing digit50:fix/msgspec-recursive-hooks (1fe118f) with main (2c6cf5b)1
Footnotes
|
Pretty sure the performance regressions are unrelated. Can you add a link to the PR to the history entry? Also I don't think this is the end of the story - for example, |
This branch has not been deployed
2 participants
closes #795
With the msgspec converter,
msgspec_attrs_unstructure_factorygenerated a class's unstructuring hook while its recursion probe still held classes in the working set. Field handlers that led back to a probed class were looked up during that time and came back asmsgspec.to_builtins, so nested instances skipped registered hooks and lost private attributes. I changed both generation points (the private-attribute path and the one after the probe) so that, while any probe is active, the factory returns a hook that generates the real one on its first call, after the outermost probe has finished. Outside a probe the hook is generated immediately as before. Returningconverter.unstructureinstead is not enough: a namedtuple field caches the hooks of its element types during the probe, and a private class reached that way would then dispatch to itself forever.The new tests in
tests/preconf/test_msgspec_cpython.pycover a self-recursive class with a hookeddatetimefield, a mutually recursive pair where one class has a private attribute, and a private class reached through a namedtuple field. The first two fail on main with the reported output.